Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

More data access tests, some refactoring and cleanup #18312

Merged
merged 29 commits into from
Jun 10, 2024
Merged

More data access tests, some refactoring and cleanup #18312

merged 29 commits into from
Jun 10, 2024

Conversation

jdavcs
Copy link
Member

@jdavcs jdavcs commented Jun 4, 2024

Replaces #17662. Builds on #18079.

More data-access unit testing. Database created and loaded once per session; cleared after each test. Dependent model instances created recursively as needed, with provided arguments or reasonable defaults. The key idea is (a) minimal setup, specifying only what's essential for a given test; and (b) auto-cleanup after each test to eliminate inter-test dependencies.

I've converted a bunch of tests from test_galaxy_mapping to use the new fixtures. The logic of the tests is unchanged, the only difference is the setup. Here's one example of what it looks like:

old

# test/unit/data/test_galaxy_mapping.py
  def test_populated_optimized_ok(self):
        u = model.User(email="[email protected]", password="password")
        h1 = model.History(name="History 1", user=u)
        d1 = model.HistoryDatasetAssociation(
            extension="txt", history=h1, create_dataset=True, sa_session=self.model.session
        )
        d2 = model.HistoryDatasetAssociation(
            extension="txt", history=h1, create_dataset=True, sa_session=self.model.session
        )
        c1 = model.DatasetCollection(collection_type="paired")
        dce1 = model.DatasetCollectionElement(collection=c1, element=d1, element_identifier="forward", element_index=0)
        dce2 = model.DatasetCollectionElement(collection=c1, element=d2, element_identifier="reverse", element_index=1)
        self.model.session.add_all([d1, d2, c1, dce1, dce2])
        self.model.session.flush()
        assert c1.populated
        assert c1.populated_optimized

new

# test/unit/data/model/db/test_misc.py
def test_populated_optimized_ok(session, make_dataset_collection, make_dataset_collection_element, make_hda):
    c1 = make_dataset_collection(collection_type="paired")
    make_dataset_collection_element(collection=c1, element=make_hda(create_dataset=True, sa_session=session))
    make_dataset_collection_element(collection=c1, element=make_hda(create_dataset=True, sa_session=session))
    assert c1.populated
    assert c1.populated_optimized

How to test the changes?

(Select all options that apply)

  • I've included appropriate automated tests.
  • This is a refactoring of components with existing test coverage.
  • Instructions for manual testing are as follows:
    1. [add testing steps and prerequisites here if you didn't write automated tests covering all your changes]

License

  • I agree to license these and all my past contributions to the core galaxy codebase under the MIT license.

@jdavcs jdavcs added kind/enhancement area/testing area/database Galaxy's database or data access layer labels Jun 4, 2024
@jdavcs jdavcs added this to the 24.2 milestone Jun 4, 2024
@jdavcs jdavcs requested a review from a team June 4, 2024 04:34
@jdavcs jdavcs mentioned this pull request Jun 4, 2024
Closed
4 tasks
jdavcs added 23 commits June 10, 2024 15:48
@jdavcs
Improve model fixtures
1c90246 
We can't pass make_foo fixture functions as the default argument to
kwd.get("foo", make_foo()) because make_foo is executed in any case,
which creates a side effect by loading a new foo row into the database,
which leads to failing tests since the number of foo records becomes off
by one.
@jdavcs
Add first library data access tests
71b9e4b 
- factor out db access from lib manager;
- add unit tests
- move data access functions from managers.libraries to model.db.libraries
@jdavcs
Use session.get instead of custom method
0dd284a
@jdavcs
Add first user data access tests
0c840d0
@jdavcs
Move+test get_users_by_ids
fd71792
@jdavcs
security.get_npns_roles: test + factor out
b8e05af
@jdavcs
security.get_private_user_role: test + factor out
fabce5b
@jdavcs
webapps.galaxy.services.user.get_users_for_index: test + factor out
4329a5e
@jdavcs
Test User.username model definition (unique constraint)
f91bd11
@jdavcs
Move+test get_roles_by_ids
4aaed3e
@jdavcs
Start replacing test_galaxy_mapping w/data access tests
7ad6d43
@jdavcs
Improve test: enable db triggers; do not verify HistoryAudit table
bc6fb89 
We do not verify the counts of rows in the HistoryAudit table because
with the db trigger enabled, those rows are created automatically.

Although it is possible to calculate the expected counts, those numbers
would be dependend on previous db statements executed in the same test,
and that calculation would be not straightforward, making the test
brittle and confusing.
@jdavcs
More test-galaxy-mapping conversions
558d836
@jdavcs
Convert test_default_disk_usage
51ffbfd
@jdavcs
Convert test_ratings
9572a93
@jdavcs
Drop test_dataset_job_relationship
7060791 
This tested SQLAlchemy:
1. create: dataset, job
2. set: dataset.job = job
3. save, then load dataset by dataset.id; verify that dataset.job = job
@jdavcs
Drop test_jobs
1a4d6e6 
Tested SQLAlchemy:
- create job
- set job.tool_id
- save, load job by id, verify job.tool_id
@jdavcs
Convert test_history_contents
da3c885
@jdavcs
Convert test_current_galaxy_sesssion
2d4a65d
@jdavcs
Convert hid tests
23ae962
@jdavcs
Convert test_get_display_name
c8df2fd
@jdavcs
Drop test_tags
47a8bf7 
Tests SQLAlchemy:

1. Create tag
2. Verify there's no FooTagAssociation with this tag
3. Add new FooTagAssociation to Foo and save to db
4. Verify there is one FooTagAssociation with this tag
@jdavcs
Drop incomplete test
83e124f 
No assert (only TODO); but would test SQLAlchemy
jdavcs added 6 commits June 10, 2024 15:48
@jdavcs
Drop test_basic
e7d4270 
Tests SQLAlchemy, user-history relationship mapping
@jdavcs
Convert test_metadata_spec
5123c39
@jdavcs
Drop unused fixture arg
65530c9
@jdavcs
Convert + improve test_job/task_metrics
1d7d655
@jdavcs
Drop test_tasks
fd29aab 
Tests SQLAlchemy (create foo + set foo.bar + load foo + check bar)
@jdavcs
Add test_history_update
0712fbf
@jdavcs jdavcs force-pushed the dev_data_access3 branch from 6dbf58a to 0712fbf Compare June 10, 2024 19:48
@jdavcs jdavcs merged commit 5b4268c into galaxyproject:dev Jun 10, 2024
52 of 53 checks passed
@jdavcs jdavcs added the highlight/dev Included in admin/dev release notes label Nov 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jmchilton jmchilton jmchilton approved these changes

Assignees
No one assigned
Labels
area/database Galaxy's database or data access layer area/testing highlight/dev Included in admin/dev release notes kind/enhancement
Projects
None yet
Milestone
24.2
Development

Successfully merging this pull request may close these issues.
2 participants
@jdavcs @jmchilton